-
Notifications
You must be signed in to change notification settings - Fork 3
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Support migration from the Firefly LNS #97
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
DEBUG Failed to determine channel index of uplink {"ack": false, "adr": false, "adr_ack_req": false, "band_id": "EU_863_870", "bandwidth": 125000, "class_b": false, "data_rate": "lora:{bandwidth:125000 spreading_factor:7 coding_rate:"4/5"}", "dev_addr": "29559735", "error": "error:pkg/networkserver:uplink_channel_not_found (uplink channel not found)", "f_cnt_reset": false, "f_opts_len": 0, "f_port": 1, "frequency": 867700000, "full_f_cnt_up": 11, "grpc.method": "HandleUplink", "grpc.service": "ttn.lorawan.v3.GsNs", "last_f_cnt_up": 10, "m_type": "UNCONFIRMED_UP", "mac_count": 0, "mac_version": "MAC_V1_0_2", "major": "LORAWAN_R1", "namespace": "networkserver", "pending_session": false, "phy_payload_len": 14, "received_at": 1698777742.7678819, "request_id": "01HE3GPFDFN4CBZX9Q697JE83A", "spreading_factor": 7, "uplink_f_cnt": 11}
This is normal to a certain extent - you don't pass any MAC state while importing the end device, which means that the LNS builds a MAC state which has the current parameters with only the 'boot' channels - 867700000
is not a 'boot' channel.
The ttnv2
source can show you how to build the MAC state and how to edit it. In this case you can do the same current = desired
trick and then work back the different parameters in Firefly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great! I like how you've streamlined the config part.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This JoinEUI thing is probably quite problematic, no? This only works if there's one JoinEUI per batch? And you either migrate one device, or you migrate all devices accessible with the API key?
I presume Firefly just takes the JoinEUI from the join-request, and uses the DevEUI as primary identifier?
So if there's a mix of JoinEUIs in a larger deployment, devices need to be migrated one by one?
logLevel := log.InfoLevel | ||
if verbose, _ := cmd.Flags().GetBool("verbose"); verbose { | ||
logLevel = log.DebugLevel | ||
} | ||
logHandler, err := log.NewZap("console") | ||
if err != nil { | ||
return err | ||
} | ||
logger = log.NewLogger( | ||
logHandler, | ||
log.WithLevel(logLevel), | ||
) | ||
rpclog.ReplaceGrpcLogger(logger) | ||
ctx = log.NewContext(ctx, logger) | ||
|
||
exportCfg.DevIDPrefix, _ = cmd.Flags().GetString("dev-id-prefix") | ||
exportCfg.EUIForID, _ = cmd.Flags().GetBool("set-eui-as-id") | ||
ctx = export.NewContext(ctx, exportCfg) | ||
|
||
cmd.SetContext(ctx) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The context was previously never propagated to the subcommands and hence this logger and the exportconfig were never used.
I've fixed the export config but we don't need this logger since we use Zap in the source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great
Yes indeed. The way I have documented this is to migrate devices in batches by creating a file with the Device EUIs and piping that into the |
Summary
Support migration from the Firefly LNS. Replaces #49.
Changes
Testing
With a The Things Uno.
This can also be checked via the UI
Regressions
NA. This is new functionality.
Notes for Reviewers
EU_863_870_TTN
instead? I'm using a The Things Uno for testing.Does anything stand out to you here @adriansmares?
Checklist
CHANGELOG.md
.